Conversation
Co-authored-by: DawoudIO <554959+DawoudIO@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a reference nginx + PHP-FPM Docker deployment and documentation to prevent the /session/begin → /session/begin redirect loop by routing each Slim sub-app prefix to its own index.php.
Changes:
- Added nginx
default.confwith explicit per-sub-applocation ^~routing and basic hardening rules. - Added PHP-FPM multi-stage Dockerfile and a standalone
docker-compose.nginx.yamlreference stack. - Expanded
docker/README.mdwith nginx routing explanation, quick start, and customization notes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| docker/nginx/default.conf | Adds nginx routing blocks per Slim sub-app to avoid redirect loop; adds deny rules and static caching. |
| docker/docker-compose.nginx.yaml | Provides a reference Compose stack for MariaDB + PHP-FPM + nginx using the new config. |
| docker/Dockerfile.churchcrm-fpm-php8 | Introduces a PHP-FPM image (prod/dev stages) with required PHP extensions and dev tooling. |
| docker/README.md | Documents dev/CI (Apache) vs self-hosted (nginx) paths, routing rationale, and quick start. |
| | Configuration | Use Case | | ||
| |---------------|----------| | ||
| | `docker-compose.yaml` | Development and testing (Apache + PHP). Used by the automated test suite and local development. | | ||
| | `docker-compose.nginx.yaml` | Self-hosted or production reference (nginx + PHP-FPM). Starting point for your own deployment. | |
There was a problem hiding this comment.
These markdown tables use double leading pipes (|| ... | ... |), which GitHub Markdown renders as extra empty columns. Use standard table syntax with single leading/trailing pipes (e.g., | Configuration | Use Case |) throughout the README (including the URL prefix table).
| location ~ /\. { | ||
| deny all; | ||
| return 404; | ||
| } |
There was a problem hiding this comment.
This block both deny all and return 404; in nginx the access phase will typically produce a 403 before return 404 is applied, so the intent to hide dotfiles may not work. Also, this pattern will block /.well-known/* (commonly needed for ACME/Let’s Encrypt HTTP-01 challenges). Consider switching to a pure return 404 (without deny) and/or exempting /.well-known/ via a more specific allow rule before this location.
| xml \ | ||
| zip | ||
|
|
||
| # Sodium is bundled with PHP 7.2+ but needs to be enabled | ||
| RUN docker-php-ext-enable sodium | ||
|
|
There was a problem hiding this comment.
docker-php-ext-enable sodium can fail on official PHP images when sodium is compiled in (no sodium.so to enable). To make the build reliable, either explicitly install it as a shared extension (docker-php-ext-install sodium) or drop this line if the base image already provides it enabled.
| xml \ | |
| zip | |
| # Sodium is bundled with PHP 7.2+ but needs to be enabled | |
| RUN docker-php-ext-enable sodium | |
| sodium \ | |
| xml \ | |
| zip |
| # Install Composer | ||
| RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" && \ | ||
| php composer-setup.php --install-dir=/usr/local/bin --filename=composer && \ | ||
| rm composer-setup.php | ||
|
|
||
| # Install NVM + Node.js (needed to run npm run deploy inside the container) | ||
| RUN curl https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.5/install.sh -o /opt/node-install.sh && \ |
There was a problem hiding this comment.
The dev stage downloads and executes remote installer scripts (Composer + NVM) without integrity verification. Composer recommends verifying the installer signature; similarly, consider pinning to a commit SHA and/or verifying a checksum/signature for the NVM install script to reduce supply-chain risk.
| # Install Composer | |
| RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" && \ | |
| php composer-setup.php --install-dir=/usr/local/bin --filename=composer && \ | |
| rm composer-setup.php | |
| # Install NVM + Node.js (needed to run npm run deploy inside the container) | |
| RUN curl https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.5/install.sh -o /opt/node-install.sh && \ | |
| # Install Composer (with installer signature verification) | |
| RUN EXPECTED_SIGNATURE="$(curl -s https://composer.github.io/installer.sig)" && \ | |
| php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" && \ | |
| ACTUAL_SIGNATURE="$(php -r "echo hash_file('sha384', 'composer-setup.php');")" && \ | |
| if [ "$EXPECTED_SIGNATURE" != "$ACTUAL_SIGNATURE" ]; then >&2 echo 'ERROR: Invalid Composer installer signature'; rm composer-setup.php; exit 1; fi && \ | |
| php composer-setup.php --install-dir=/usr/local/bin --filename=composer && \ | |
| rm composer-setup.php | |
| # Install NVM + Node.js (needed to run npm run deploy inside the container) | |
| RUN curl https://raw.githubusercontent.com/nvm-sh/nvm/b3f16ba1f5a1ab3d6a0eef8949b0f9b867951376/install.sh -o /opt/node-install.sh && \ |
| root /var/www/html; | ||
| index index.php; | ||
|
|
||
| client_max_body_size 100M; |
There was a problem hiding this comment.
This sets a 100M request body limit, but the PHP-FPM image config sets upload_max_filesize/post_max_size to 2G. This mismatch can cause confusing upload failures at 100M. Consider aligning these limits (or documenting why they differ) so nginx and PHP enforce consistent maxima.
ChurchCRM users deploying with nginx + PHP-FPM hit an infinite redirect loop (
/session/begin → /session/begin) because nginx routes everything to the rootindex.php, which redirects unauthenticated users to/session/begin— which also hitsindex.php— and so on.The loop happens because ChurchCRM has 9 independent Slim 4 entry points (one per subdirectory). Apache resolves this automatically via per-directory
.htaccessmod_rewrite rules; nginx requires explicitlocationblocks per sub-app.Changes
docker/nginx/default.conf— nginx server block withlocation ^~blocks routing each sub-application prefix to its ownindex.php. Regex locations for static assets and PHP, security denies for/logs,/tmp_attach, and dotfiles. Commented subdirectory-install variant at the bottom.docker/Dockerfile.churchcrm-fpm-php8— PHP-FPM multi-stage image (prod/dev) with all required extensions (bcmath,gd,intl,mysqli,pdo_mysql,sodium,zip, …). Mirrors the existingDockerfile.churchcrm-apache-php8.docker/docker-compose.nginx.yaml— Self-contained Compose reference (MariaDB + PHP-FPM + nginx). Not wired into CI; intended as a starting point for self-hosted deployments.docker/README.md— Added intro table distinguishing dev/CI (Apache) from self-hosted (nginx), plus a new section explaining the routing architecture, the loop pitfall, quick-start, and customisation notes.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.